[jaxrs-spec][quarkus] Emit @RolesAllowed({"**"}) for HTTP Basic, Bearer, api-key and OAuth2 or OpenID with empty scopes and rename "useQuarkusSecurityAnnotations" to "useJakartaSecurityAnnotations"#23680
Open
Ignacio-Vidal wants to merge 13 commits intoOpenAPITools:masterfrom
Conversation
7479247 to
5daad84
Compare
5daad84 to
9b2aa6e
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="quarkus-security-github-issue.md">
<violation number="1" location="quarkus-security-github-issue.md:59">
P1: OpenAPI scope requirements are conjunctive, but this mapping uses `@RolesAllowed` OR semantics and can under-enforce multi-scope authorization.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| | `apiKey` | `@Authenticated` | An API key validates identity only. No role check is applicable. | | ||
| | `oauth2` with empty scopes (`[]`) | `@Authenticated` | An empty scope list means "any authenticated user" — no specific authorization is required beyond a valid token. | | ||
| | `openIdConnect` with empty scopes (`[]`) | `@Authenticated` | Same reasoning as OAuth2 with empty scopes. | | ||
| | `oauth2` with explicit scopes | `@RolesAllowed({"scope1", "scope2"})` | In Quarkus, OAuth2/OIDC token scopes are mapped to `SecurityIdentity` roles. `@RolesAllowed` receives the scopes as role names. | |
Contributor
There was a problem hiding this comment.
P1: OpenAPI scope requirements are conjunctive, but this mapping uses @RolesAllowed OR semantics and can under-enforce multi-scope authorization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At quarkus-security-github-issue.md, line 59:
<comment>OpenAPI scope requirements are conjunctive, but this mapping uses `@RolesAllowed` OR semantics and can under-enforce multi-scope authorization.</comment>
<file context>
@@ -0,0 +1,209 @@
+| `apiKey` | `@Authenticated` | An API key validates identity only. No role check is applicable. |
+| `oauth2` with empty scopes (`[]`) | `@Authenticated` | An empty scope list means "any authenticated user" — no specific authorization is required beyond a valid token. |
+| `openIdConnect` with empty scopes (`[]`) | `@Authenticated` | Same reasoning as OAuth2 with empty scopes. |
+| `oauth2` with explicit scopes | `@RolesAllowed({"scope1", "scope2"})` | In Quarkus, OAuth2/OIDC token scopes are mapped to `SecurityIdentity` roles. `@RolesAllowed` receives the scopes as role names. |
+| `openIdConnect` with explicit scopes | `@RolesAllowed({"scope1"})` | Same as OAuth2 with scopes. |
+| OR list with at least one empty-scope scheme | `@Authenticated` | The least restrictive alternative dominates: if any scheme allows any authenticated user, the effective gate is authentication only. |
</file context>
Contributor
Author
There was a problem hiding this comment.
the generator handles joint and disjoint sets of security schemes added in the openapi specification
e92ab2e to
b3b43db
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is part 2 of #23691 to improve authentication and authorisation support in the `jaxrs-spec/quarkus` server generator.
What this PR does
When the new
useJakartaSecurityAnnotationsflag is enabled (Quarkus library only), the generator emits@jakarta.annotation.security.RolesAllowed({"**"})on JAX-RS interface methods and implementation stubs for operations whose security requirements mean any authenticated user is sufficient — i.e. no specific role or scope is enforced by the token.Schemes that qualify
scheme: [])OR semantics — at least one alternative qualifies
The OpenAPI
securityarray is OR: a request is authorised if any one of the listed alternatives is satisfied. The annotation is emitted when at least one OR alternative fully qualifies on its own:AND semantics — all co-required schemes must qualify
A single
SecurityRequirementobject with multiple keys is AND: all listed schemes must be satisfied simultaneously. If any scheme in the AND group carries explicit scopes, the combined requirement is more restrictive than "any authenticated user", so the annotation is not emitted:Why the annotation is not emitted when scopes are present
@RolesAllowed({"**"})means any authenticated principal. If scopes are present, the intent is to restrict to principals holding a specific role. Emitting@RolesAllowed({"**"})in that case would be too permissive and contradict the spec. Those operations are left unannotated pending a follow-up PR that will emit@RolesAllowed({scope})for the all-scoped case.Emitting both annotations on the same method is not a valid fallback: Quarkus applies them with AND semantics, making
@RolesAllowed({"**"})redundant at best and silently incorrect at worst.Implementation notes
Why
fromOperationand notpostProcessOperationsWithModelsThe OpenAPI
securityarray uses aList<SecurityRequirement>where each element is a map of scheme name → scope list. A single map entry with multiple keys is an AND group. By the timepostProcessOperationsWithModelsruns,DefaultGenerator.filterAuthMethodshas already flattened allSecurityRequirementobjects into a plainList<CodegenSecurity>, discarding which schemes were co-located in the same AND group. Evaluating mixed-scope AND groups correctly requires the raw structure, which is still available infromOperationvia theOperationparameter. The inheritedprotected OpenAPI openAPIfield provides access to the security scheme definitions needed to resolve scheme types by name.Vendor extension
fromOperationsetsx-jakarta-any-roles = trueon qualifying operations. The Mustache templates for bothapiInterface.mustache(interface-only mode) andapiMethod.mustache(implementation stub mode) emit the annotation inside a{{#vendorExtensions.x-jakarta-any-roles}}block.Migration
Replace
useQuarkusSecurityAnnotationswithuseJakartaSecurityAnnotationsso the support for authentication and authorisation annotations can be extended to other libraries under the same generator. #23699 added the initialuseQuarkusSecurityAnnotationsannotation to master and has not been released in a tag version yetGenerated code now uses
@jakarta.annotation.security.RolesAllowed({"**"})instead of@io.quarkus.security.Authenticated.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)